[Reporting] Update more Server Types for TaskManager #74915

tsullivan · 2020-08-12T23:35:08Z

Summary

Pulling changes out of #64853

While working on removing ESQueue and using Task Manager, I found opportunistic ways to make the server types more strict, and allow some of the main modules such as ReportingStore to handle more corner cases. This PR takes out the opportunistic work out as a standalone PR.

List of changes:

Remove the ReportingCore.enqueueJob method since the route handler can just import enqueueJobFactory
Rename ESQueueCreateJobFn to CreateJobFn
Rename ESQueueWorkerExecuteFn to WorkerExecuteFn
Check the cancellation token in a loop in generate_csv
Define CreateJobBaseParams and CreateJobBaseParamsEncryptedFields to add stricter typing on job payloads (way less unknown in the system)
Remove ReportingStore.updateReport and add setReportClaimed, setReportCompleted, setReportFailed
Add some JSON formatting methods to the Report class
Make a copy of the ESQueue constants/statuses file for use outside of ESQueue

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-08-13T16:04:44Z

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

tsullivan · 2020-08-13T16:05:03Z

@elasticmachine merge upstream

joelgriffith · 2020-08-13T16:52:40Z

x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts

@@ -113,6 +113,10 @@ export function createGenerateCsv(logger: LevelLogger) {
          break;
        }

+        if (cancellationToken.isCancelled()) {


This is an interesting one... wonder how we missed it for so long

joelgriffith · 2020-08-13T16:57:19Z

x-pack/plugins/reporting/server/lib/enqueue_job.ts

    const exportType = reporting.getExportTypesRegistry().getById(exportTypeId);

    if (exportType == null) {
      throw new Error(`Export type ${exportTypeId} does not exist in the registry!`);
    }

+    // encrypt the headers in the payload


Small nit: you can combine both 46 and 48 into a parallel await

const [ scheduleTask, { store }, ] = await Promise.all([ exportType.scheduleTaskFnFactory(reporting, logger) as ScheduleTaskFnType, reporting.getPluginStartDeps() ]);

x-pack/plugins/reporting/server/lib/esqueue/constants/index.js

joelgriffith · 2020-08-13T17:07:18Z

x-pack/plugins/reporting/server/lib/store/mapping.ts

@@ -45,7 +45,7 @@ export const mapping = {
  priority: { type: 'byte' },
  timeout: { type: 'long' },
  process_expiration: { type: 'date' },
-  created_by: { type: 'keyword' },
+  created_by: { type: 'keyword' }, // `null` if security is disabled


Good callout

Yeah, I think it used to be false but it sneakily changed through TS improvements. That shouldn't be a problem for mappings, I think.

joelgriffith

Some small nits/perf improvements

tsullivan · 2020-08-13T18:55:54Z

@joelgriffith I pushed up a commit per your feedback.

I also removed some more type-casting in the routes, because I couldn't resist thanks to the help from #74879

kibanamachine · 2020-08-13T20:52:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9ae80c0

Build metrics

✅ unchanged

History

💚 Build #67960 succeeded 30111ea
💚 Build #67860 succeeded c7706e7
💔 Build #67843 failed 4861da3

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

joelgriffith

Thanks for the context and cleanup of types! LGTM

joelgriffith · 2020-08-13T21:21:55Z

x-pack/plugins/reporting/server/routes/generate_from_jobparams.ts

-        jobParamsRison = jobParamsPayload;
-      } else {
-        const { jobParams: queryJobParams } = req.query as { jobParams: string };
+        const { jobParams: jobParamsPayload } = req.body;


yissssss that cleanup

* [Reporting] Update more Server Types for TaskManager * remove some task manager references * more strict * more strict 2 * simplify * fix test * fix test * routing validation unused types cleanup * remove more casting in route handlers * feedback changes * original comment was fine Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/reporting/server/export_types/printable_pdf/create_job/index.ts # x-pack/plugins/reporting/server/export_types/printable_pdf/types.d.ts # x-pack/plugins/reporting/server/lib/store/store.ts

* master: (23 commits) Adding API test for custom link transaction example (elastic#74238) [Uptime] Singular alert (elastic#74659) Revert "attempt excluding a codeowners directory" (elastic#75023) [Metrics UI] Remove TSVB dependency from Metrics Explorer APIs (elastic#74804) Remove degraded state from ES status service (elastic#75007) [Reporting/Functional] unskip pagination test (elastic#74973) [Resolver] Stale query string values are removed when resolver's component instance ID changes. (elastic#74979) Add public url to Workplace Search plugin (elastic#74991) [Reporting] Update more Server Types for TaskManager (elastic#74915) [I18n] verify select icu-message options are in english (elastic#74963) Make data.search.aggs available on the server. (elastic#74472) [Security Solution][Resolver] Graph Control Tests and Update Simulator Selectors (elastic#74680) attempt excluding a codeowners directory [ML] DF Analytics: allow failed job to be stopped by force via the UI (elastic#74710) Add kibana-core-ui-designers team (elastic#74970) [Metrics UI] Fix inventory footer misalignment (elastic#74707) Remove legacy optimizer (elastic#73154) Update design-specific GH code-owners (elastic#74877) skip test Reporting paginates content elastic#74922 [Metrics UI] Add Jest tests for alert previews (elastic#74890) ...

…75003) * [Reporting] Update more Server Types for TaskManager (#74915) * [Reporting] Update more Server Types for TaskManager * remove some task manager references * more strict * more strict 2 * simplify * fix test * fix test * routing validation unused types cleanup * remove more casting in route handlers * feedback changes * original comment was fine Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/reporting/server/export_types/printable_pdf/create_job/index.ts # x-pack/plugins/reporting/server/export_types/printable_pdf/types.d.ts # x-pack/plugins/reporting/server/lib/store/store.ts * fix test * fix test

[Reporting] Update more Server Types for TaskManager

0ff98f6

tsullivan mentioned this pull request Aug 12, 2020

Switch Reporting to Task Manager #64853

Merged

7 tasks

tsullivan changed the title ~~Reporting/cleanup job param types~~ [Reporting] Update more Server Types for TaskManager Aug 12, 2020

tsullivan added release_note:skip Skip the PR/issue when compiling release notes Team:Reporting Services v7.10.0 v8.0.0 labels Aug 12, 2020

tsullivan added 7 commits August 12, 2020 16:51

remove some task manager references

cab5de4

more strict

1c76cbd

more strict 2

e9a809e

simplify

4861da3

fix test

f111bcb

fix test

7a446e8

routing validation unused types cleanup

c7706e7

tsullivan requested a review from joelgriffith August 13, 2020 16:04

tsullivan marked this pull request as ready for review August 13, 2020 16:04

Merge branch 'master' into reporting/cleanup-job-param-types

30111ea

joelgriffith reviewed Aug 13, 2020

View reviewed changes

x-pack/plugins/reporting/server/lib/esqueue/constants/index.js Show resolved Hide resolved

joelgriffith reviewed Aug 13, 2020

View reviewed changes

joelgriffith suggested changes Aug 13, 2020

View reviewed changes

tsullivan added 4 commits August 13, 2020 10:30

Merge branch 'master' into reporting/cleanup-job-param-types

bb5fbd0

remove more casting in route handlers

dcacb6d

feedback changes

7a9175c

original comment was fine

9ae80c0

joelgriffith approved these changes Aug 13, 2020

View reviewed changes

tsullivan merged commit a8ed1f4 into elastic:master Aug 13, 2020

tsullivan mentioned this pull request Aug 13, 2020

[7.x] [Reporting] Update more Server Types for TaskManager (#74915) #75003

Merged

This was referenced Aug 18, 2020

Reporting page doesn't show any reports. User doesn't get report completed notifications #75239

Closed

[Reporting/JobListing] fix user ID for non-security in queries #75365

Merged

tsullivan deleted the reporting/cleanup-job-param-types branch September 3, 2020 22:46

sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting] Update more Server Types for TaskManager #74915

[Reporting] Update more Server Types for TaskManager #74915

tsullivan commented Aug 12, 2020 •

edited

Loading

elasticmachine commented Aug 13, 2020

tsullivan commented Aug 13, 2020

joelgriffith Aug 13, 2020

joelgriffith Aug 13, 2020 •

edited

Loading

joelgriffith Aug 13, 2020

tsullivan Aug 13, 2020

joelgriffith left a comment

tsullivan commented Aug 13, 2020

kibanamachine commented Aug 13, 2020

joelgriffith left a comment

joelgriffith Aug 13, 2020

[Reporting] Update more Server Types for TaskManager #74915

[Reporting] Update more Server Types for TaskManager #74915

Conversation

tsullivan commented Aug 12, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Aug 13, 2020

tsullivan commented Aug 13, 2020

joelgriffith Aug 13, 2020

Choose a reason for hiding this comment

joelgriffith Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

joelgriffith Aug 13, 2020

Choose a reason for hiding this comment

tsullivan Aug 13, 2020

Choose a reason for hiding this comment

joelgriffith left a comment

Choose a reason for hiding this comment

tsullivan commented Aug 13, 2020

kibanamachine commented Aug 13, 2020

💚 Build Succeeded

Build metrics

History

joelgriffith left a comment

Choose a reason for hiding this comment

joelgriffith Aug 13, 2020

Choose a reason for hiding this comment

tsullivan commented Aug 12, 2020 •

edited

Loading

joelgriffith Aug 13, 2020 •

edited

Loading